Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add windows disclaimer and change int to Integer. #26

Merged
merged 4 commits into from
Mar 3, 2024

Conversation

lupemba
Copy link
Contributor

@lupemba lupemba commented Feb 21, 2024

I have tried to fix the issue I encountered with Int32, see issue #25

This does still not work completely since I am using Windows. I have therefore added a notice to the readme about the Windows support. It is easy to miss this when it is only written on GRIB.jl

@tcarion tcarion marked this pull request as ready for review February 23, 2024 08:43
@tcarion
Copy link
Member

tcarion commented Feb 23, 2024

@lupemba Thank you very much for the PR ! I took advantage of this to adapt the types for some other functions. This is actually legacy code from Cfgrib.jl that I didn't really double check.

Are you using GRIBDatasets.jl with GRIB.jl v4.0.0 ? Because I saw they added some support for windows, so this should be reflected here

@lupemba
Copy link
Contributor Author

lupemba commented Feb 23, 2024

Hi @tcarion,

I use the new version and I can see that they have added some windows support. The package tests still fails on windows because GRIB.Index is not defined for windows systems.

Yesterday I discovered that my issues is not with windows but instead that the .grib file has two longitude and two latitude dimensions. Two different grids in one file. I tried to remove the length check from getone(index::FileIndex, key::AbstractString) and was able to get the following out.

image

I am planning to make a new issue on this but I am just downloading a small file to reproduce it.

@tcarion
Copy link
Member

tcarion commented Feb 23, 2024

I use the new version and I can see that they have added some windows support. The package tests still fails on windows because GRIB.Index is not defined for windows systems.

Mhm, I see. It should actually be possible to do without Index in GRIBDatasets.jl if we are on Windows, I don't think this should be difficult to do. I'll try when I have time :-)

Two different grids in one file. I tried to remove the length check from getone(index::FileIndex, key::AbstractString) and was able to get the following out.

Unfortunaltely, I think it's a untrivial issue to solve. I see 2 workarounds:

  • Filter on each variable from GRIBDatasets.jl. For example: GRIBDataset(grib_path; filter_by_values = Dict("cfVarName" => "sst")). But this means you will need to treat each variable separately...
  • Create 2 grib files separating the variables defined on distinct grids. But this requires a separate GRIB utility (like https://confluence.ecmwf.int/display/ECC/grib_filter)

Anyway, that'd be great if you can open an issue with a MWE :-)

@lupemba
Copy link
Contributor Author

lupemba commented Feb 23, 2024

Thank you for making the package @tcarion. It is nice that we now can work with grib files in Julia.

Anyway, that'd be great if you can open an issue with a MWE :-)

My download from ECMWF is still queued. I hope to create the issue on Monday.

@tcarion tcarion merged commit ba9a735 into JuliaGeo:main Mar 3, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants